I'm not convinced this is a common enough use case that it has to be in facade, couldn't it be implemented in the logging implementation?

@Thomasdezeeuw

spdlog-rs implements this, with their own log! macro. When adapting to the log crate's macros, the functionality can't be used.

So, the logging implementation must implement the same functionality by bypassing our macro, which loses the value of a facade. If you have an idea how to implement and use it with our abstraction and a custom impl crate, I'm glad to know.

So, the logging implementation must implement the same functionality by bypassing our macro, which loses the value of a facade. If you have an idea how to implement and use it with our abstraction and a custom impl crate, I'm glad to know.

I my crate I used the target to change the target of the logs. In this it uses a constant for the target to switch from logging to standard error to standard out. See https://docs.rs/std-logger/latest/std_logger/macro.request.html for an example, but any macro can be used by setting the target to a constant (https://docs.rs/std-logger/latest/std_logger/constant.REQUEST_TARGET.html)

Thanks for your information! I'm considering the difference between target and a logger field also.

In the description:

I noticed that we previously introduced target for similar usage, but switching to a different logger is different.

Switching targets still use the same (global & singleton) logger instance, while setting a new logger instance can avoid meta logging cycle - #645.

Switching targets still use the same (global & singleton) logger instance, while setting a new logger instance can avoid meta logging cycle - #645.

The global logging implementation can be set up to call different loggers depending on target.

Personally, I think being able to target a different logger entirely using an argument is useful in a few scenarios:

1. You want to log from your logging pipeline. If your pipeline is complex enough, you sometimes end up needing to do this.
2. You don't want to use a global logger at all, but still want some of the nice ergonomics of the macros. You may do this, for example, to implement your own simplified contextual logging.

From a maintenance perspective it also keeps us a bit honest in not being able to assume that everything is shared by a single global logger.

If @Thomasdezeeuw isn't convinced it's worth adding though then we'll leave it for the time being.

I'm not convinced the additional complexity is worth the use case that is already possible via the log implementation.

@tisonkun do you have any argument why it should be done this way, and not via the log implementation?

It's included in the issue description of #645:

Filter certain targets in specific appended:

impl log::Log KafkaAppender {
  fn log(&self, record: &Record) {
    if record.target().start_with("rdkafka") { return; }
  }
}
```

This is, however, both brittle and may filter logs from user code. I don't know how targets are defined and if we can reliably depend on that matches.

That said, passing a logger instance is determinate for what is installed in the log implementation (pipeline), while target is a loose contract (convention).

If you have a single log instance with a Kafka appender installed, you may first filter out logs with target "kafka". But what if a user application uses the same target for their actions over kafka? Then you need to agree on a reserved target and every user should learn about it.

Instead, a pass-in logger can be an explicit contract that the certain logger is used by kafka client internally (if a client library adopts this) and thus you can fine tune the manner and be sure that it would be used in that scenario.

You don't want to use a global logger at all, but still want some of the nice ergonomics of the macros. You may do this, for example, to implement your own simplified contextual logging.

This is also a good point that when you have different log requirements for different components, instead of making a global instance and dispatching the application logic (rather than barely appending to multiple destinations), it would be easier to keep the logger instance separated.

Finally, if we support logging with a pass-in instance, it's possible to use a global instance everywhere. But the reverse direction is not true. slog's macros always has a logger param, while there is slog-global to use a global instance when it's satisfied.

This PR reminds me of with_default:

Sets this Subscriber as the default for the current thread for the duration of a closure.

The default subscriber is used when creating a new Span or Event.

Sounds like you actually want something like this?

@tisonkun

Sounds like you actually want something like this?

@NobodyXu No. This is a thread-local default, which can be even worse than a global default to tune with. The logger field in log! macro is an explicit instance, with_default is a more subtle implict.

Merge master branch and resolve conflicts.

May we have another round to review this patch?

@Thomasdezeeuw are you happy if we move forwards with this feature? I think pushing back on additions is a good position that keeps us from accumulating bloat, but feel like this one specifically is a reasonable enhancement aligned with the existing macro capabilities.

I still think the use case can be solved without this change, hence I don't think it's worth the additional complexity to be honest.

However, if we go forward with it have we looked at the code size changes? Work has been done in the past to reduce the compiler code size, is that affected at all? /cc @EFanZh in case you're still interested in this

I still think the use case can be solved without this change

Some of them can, but some can be done more easily without needing to write dispatching logic based on targets. Being able to plug a logger in to the macro calls would let you use local loggers for contextual logging, without needing a centrally synchronized place to store them. That could also potentially be solved with the key-value support though if we let you plug in an additional Source alongside specific key-values.

Maybe we can think of it more like print! vs write!. The former doesn't let you specify where the output goes, whereas the latter does. If we want to decide that log! is just print! with a configurable output via set_boxed_logger then this change isn't something we'd pursue. If we want the log! macros to be a general interface to any Log impl, then I think it would be very useful.

I do this we should add some more documentation for this change as I don't see any. I would at least expect the logger argument to be documented and stating that by default it will use the global logger.

Thanks for your suggestions. Then please leave me a few days so that I can pick up this patch :D

Another thought: with the current implementation, user specified logger will always be passed by reference, which might not be ideal for binary size. Do we want to support the case where user can specify a zero-sized logger to reduce binary size?

Another thought: with the current implementation, user specified logger will always be passed by reference

What does it mean? We already accept L that can be an owned logger. Could you give a concrete example where a performance concern can occur.

Does 2bb47d6 implements what you mean for passing a zero-sized logger? If not, why.

@tisonkun: Oh, I thought the implementation still passes reference for user specified loggers. Now the implementation passes logger argument as is, then I have no further questions.

@EFanZh OK. Good to know the clarification.

Then just leave me some time to figure out how to improve the docs :D